Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extend industry/subsectors to explicitly track FE by SE origin #1620

Conversation

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member

This extends industry/subsectors to explicitly track the SE-origin on FE demand. So far, we assumed identical SE-origin shares in FE demand across industry subsectors in order to report them (REMIND itself did not much care), but this collided with the assumptions made about chemical feedstocks (see #1561, #1576).

Before, we equated total industry FE supply (summed over SE origins) with industry FE demand (summed over subsectors). Now we introduce a new variable, v37_demFeIndst(t,regi,entySe,entyFe,in,emiMkt), for industry FE demand that is explicit in FE carrier, SE origin, and industry subsector (and emissions market). This allows for o37_demFeIndSub to report SE-specific subsector FE demand, without assuming SE-shares in FE carriers.

Two calibrations with these changes (SSP2EU-NPi and SSP2EU-EU21-NPi) are running under /p/tmp/pehl/Remind/ (_fast variants on the priority queue, others on medium).

  • New feature

Checklist:

  • My code follows the coding etiquette
  • I performed a self-review of my own code
  • I explained my changes within the PR, particularly in hard-to-understand areas
  • All automated model tests pass (FAIL 0 in the output of make test)
  • The changelog CHANGELOG.md has been updated correctly

vm_demFENonEnergySector(t,regi,entySe2,entyFe,"indst",emiMkt)
)
=e=
q37_feedstocksLimit(t,regi,entySe,entyFe,in,secInd37,emiMkt)$(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this restriction with the new formulation of q37_demFeFeedstockChemIndst?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which one? The rats-nest of lines 244-252? Actually, yes. This can be cleaned up by introducing some specialised mappings for the purpose, but the beauty treatment is scheduled for next week ;)

@mellamoSimon
Copy link
Contributor

awesome! thank you for this, I think it will make things more tractable

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

@JakobBD PBS is widely infeasible with this (/p/tmp/pehl/Remind/output/SSP2EU_PBS-NPi-calibrate_fast_2024-03-15_16.45.24). Sorry about that, but need to wrap my head around those equations first and I am prioritising the "default" runs for now. I will look into that next week.

@JakobBD
Copy link
Contributor

JakobBD commented Mar 15, 2024

@JakobBD PBS is widely infeasible with this (/p/tmp/pehl/Remind/output/SSP2EU_PBS-NPi-calibrate_fast_2024-03-15_16.45.24). Sorry about that, but need to wrap my head around those equations first and I am prioritising the "default" runs for now. I will look into that next week.

What does that mean for the merging process? As it's a new feature, it may not have to be merged to develop right away? Do you think you could make the PR a WIP, do your runs in your own branch and wait with merging until the pbs part is sorted out?

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

Well, it is a new feature, but one that (hopefully) solves two problems – feedstock composition and solver performance.
I would not merge before the test runs come back good (or better then the status quo) and the PBS issue is sorted. I cannot say how desperate @LaviniaBaumstark is, though.

AND secInd37_2_pf(secInd37,in)
AND secInd37_emiMkt(secInd37,emiMkt)
AND entyFe2Sector(entyFe,"indst")
AND entyFeCC37(entyFe) ) ..
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untitled

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entyFeCC37 one might be redundant. I did not want so spend a test run on finding out.

Copy link
Contributor

@JakobBD JakobBD Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is ppfen_industry_dyn37(in), made redundant by in_chemicals_feedstock_37(in), I suppose. But I wasn't even referring that, just looking in awe at 9 conditions...

AND secInd37_emiMkt(secInd37,emiMkt) ) ..
sum((sefe(entySe,entyFe),
fe2ppfEn(entyFe,in)),
v37_demFeIndst(t,regi,entySe,entyFe,in,emiMkt)
Copy link
Contributor

@JakobBD JakobBD Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's actually quite a fundamental problem: ppfen do not exist any more for process-based sectors, so the whole variable definition (and thus the FE balance) is invalid for them.
Could you maybe define v37_demFeIndst(t,regi,entySe,entyFe,secInd37,emiMkt) instead? That would still give you the full information, except for the information of primary vs secondary steel. Otherwise, you'd have to skip that intermediate step for pbs and add its share directly to q37_demFeIndst - would be a shame to not have that tracking for pbs, and I don't know what implications that would have for reporting.
Sorry about that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bottom line: PR needs significant changes in order to not break the process-based implementation. @LaviniaBaumstark

@JakobBD
Copy link
Contributor

JakobBD commented Mar 15, 2024

Nice stuff otherwise! Sorry about the pbs hassle!

@LaviniaBaumstark
Copy link
Member

many thanks for trying to improve teh situation! I agree that this is not ready for merging. Let's try to solve it on Monday and I will start calibration runs afterwards and we see how far we get for the validation. Maybe we will try to find a new date for a validation meeting before easter.

@LaviniaBaumstark
Copy link
Member

Michaja's test runs seem to converge

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented Mar 19, 2024

Test runs here

/p/tmp/pehl/Remind_twin/output/SSP2EU-NPi-calibrate_2024-03-19_10.21.34/
/p/tmp/pehl/Remind_twin/output/SSP2EU-EU21-NPi-calibrate_2024-03-19_10.21.54/
/p/tmp/pehl/Remind_twin/output/SSP2EU_PBS-NPi-calibrate_2024-03-19_10.22.11/

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

Cleaning of conditions and sets fill be a follow-up.

fe2mat(all_enty,all_enty,all_te) "" / /
secInd37_tePrc(secInd37,tePrc) "" / /
fe2ppfen_no_ces_use(all_enty,all_in) "" / /
$endif.cm_subsec_model_steel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is that needed for? Seems hard to maintain (whoever adds a set needs to know/remember there's this list lurking down here that it needs to be added to as well...)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, GAMS will help with a friendly compilation error in case something needs to be added here. :)
Only the sets used in set calculations down below, or somewhere else in the code, are required to be here. I just did not want to figure out one-by-one which those are.

Copy link
Contributor

@JakobBD JakobBD Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you got rid of all the conditions above! Please revert that! Two reasons:

  • The one on maintainability I posted above
  • There will be other process-based subsectors soon. The sets are for all subsectors, the entries are subsector-specific. Your implementation will not allow switching implementations for sectors individually.

*** be removed once the variable is established.
loop ((t,regi,
sefe(entySe,entyFe),
fe2ppfEn(entyFe,in),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may need adaptation to ue?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory yes, but testOneRegi was not screaming bloody murder, so probably it is irrelevant now.

@JakobBD
Copy link
Contributor

JakobBD commented Mar 19, 2024

Looks good, thanks for the work, let's see what the calibration says.

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q 0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q force-pushed the fix/ind_feedstock_shares branch 5 times, most recently from 2d8190f to 7a6a0ca Compare March 21, 2024 09:21
- limit the changes in prices for selected production factors to between
  0.5-2 of the price of the previous calibration iteration
- this might stabilise calibration behaviour against sudden price jumps
…ead of ppfen to be compatible wit process-based implemen-based implementation
@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

Status update

  • CES runs:
    • speed ✔️: SSP2EU-NPi and SSP2EU-EU21-NPi calibrations look pretty fast, having completed eight and six calibration iterations in 17 hours. They might actually finish on the priority queue.
    • buildings ✖️: fesob in EUR did not improve. CAZ seems to have the same problem, so more price debugging is necessary.
    • industry ❓: looks really bad in EUR, JPN, NEU, USA, but is still oscillating, so with more calibration iterations it might settle. Unfortunately I did start these runs from stock gdxes, not ones from previous calibration attempts.
  • PBS run ✖️:
    • SSP2EU_PBS-NPi on the other hand has not finished the first calibration iteration within 17 hours. It did 54 Nash iterations (SSP2EU-NPi did 26 in two and a half hours in the first calibration iteration), so this looks like a Nash convergence problem.
      image
      That might go away if we use a gdx from the other calibration, or it might not.

I would merge this now, and tinker with calibration and Nash performance based on the develop branch again. @LaviniaBaumstark?

@LaviniaBaumstark
Copy link
Member

I agree with merging and further working on improving the develop branch. Could you please also increase the input data version to 6.74

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q 0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q merged commit 75039ce into remindmodel:develop Mar 22, 2024
2 checks passed
@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

Could you please also increase the input data version to 6.74

I cannot.

cfg$inputRevision <- "6.74"

@LaviniaBaumstark
Copy link
Member

eeh, 6.75, but will do it later

@Renato-Rodrigues
Copy link
Member

It does not solve the issue of slow convergence, but on the PBS run, maybe you could try to run the calibration relaxing the tax convergence criteria at first.
As it is a new structure, and we just recently enforced the tax criteria, maybe at initial calibration stages for a new structure we are asking too much from our framework.
Once an okish gdx is achieved, we could rerun the calibration with the tax criteria reenabled.

@Renato-Rodrigues
Copy link
Member

PS: it would be nice to the person that is developing the PBS version to check also the gdx from the iteration that got infeasible running some testoneregi runs.
This iteration could give insight on which part of the model/region is actually difficult to solve on the formulation, and maybe point to the issue of slow convergence.

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

PS: it would be nice to the person that is developing the PBS version to check also the gdx from the iteration that got infeasible running some testoneregi runs. This iteration could give insight on which part of the model/region is actually difficult to solve on the formulation, and maybe point to the issue of slow convergence.

I do not think there is much information to be gained. In all likelihood, CONOPT just ran into some NA.

@0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q
Copy link
Member Author

As expected, cannot replicate.

@mellamoSimon mellamoSimon added the Chemicals Collection of PRs relevant for our new colleague modeling the chemical industry label Apr 3, 2024
JakobBD added a commit to JakobBD/remind that referenced this pull request Apr 8, 2024
…5mncnisTJJ6q/fix/ind_feedstock_shares"

This reverts commit 75039ce, reversing
changes made to b6b0e58.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chemicals Collection of PRs relevant for our new colleague modeling the chemical industry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants